Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(regression): Support input path globbing in Go re-write #23

Merged
merged 9 commits into from
Jun 4, 2024

Conversation

jsirianni
Copy link
Member

@jsirianni jsirianni commented May 23, 2024

The input file path is supported but is not working after the Go re-write v1.4.0.

Tests were added to confirm the failure, they pass now.

  • Test multiple files with glob
  • Test single file with glob

Re-wrote validateFilePaths and updated decodeAnyResourceFile functions to support glob matches before reading resources. The action is validation-heavy, we have two layers of protection here.

filepath.Glob can return nil matches and a nil error, so we always need to check if the matches are nil. If it is, it could indicate an io error in rare cases. Glob will only return an error if the match pattern is invalid. https://pkg.go.dev/path/filepath#Glob

This change is backward compatible.

Testing

My test repo is passing, using globbing for the configuration path. https://github.com/jsirianni/bindplane-op-action-test/actions/runs/9207348741/job/25328061583

{"level":"info","time":"2024-05-23T12:05:20.990Z","msg":"Resource applied","name":"k8s-cluster","id":"k8s-cluster","kind":"Configuration","status":"unchanged"}
{"level":"debug","time":"2024-05-23T12:05:20.990Z","msg":"Configuration resource added to state","name":"k8s-cluster"}
{"level":"info","time":"2024-05-23T12:05:20.990Z","msg":"Applied resource","name":"k8s-cluster","status":"unchanged"}
{"level":"info","time":"2024-05-23T12:05:20.990Z","msg":"Resource applied","name":"k8s-gateway","id":"k8s-gateway","kind":"Configuration","status":"unchanged"}
{"level":"debug","time":"2024-05-23T12:05:20.990Z","msg":"Configuration resource added to state","name":"k8s-gateway"}
{"level":"info","time":"2024-05-23T12:05:20.990Z","msg":"Applied resource","name":"k8s-gateway","status":"unchanged"}
{"level":"info","time":"2024-05-23T12:05:20.990Z","msg":"Resource applied","name":"k8s-node","id":"k8s-node","kind":"Configuration","status":"unchanged"}
{"level":"debug","time":"2024-05-23T12:05:20.990Z","msg":"Configuration resource added to state","name":"k8s-node"}
{"level":"info","time":"2024-05-23T12:05:20.990Z","msg":"Applied resource","name":"k8s-node","status":"unchanged"}
{"level":"info","time":"2024-05-23T12:05:21.174Z","msg":"No pending rollout","name":"k8s-cluster"}
{"level":"info","time":"2024-05-23T12:05:21.210Z","msg":"No pending rollout","name":"k8s-gateway"}
{"level":"info","time":"2024-05-23T12:05:21.245Z","msg":"No pending rollout","name":"k8s-node"}

@@ -241,7 +241,7 @@ func (a *Action) Apply() error {
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were logging the wrong resource type here, just cosmetic.

@@ -491,25 +491,41 @@ func (a *Action) WriteBack() error {

// decodeAnyResourceFile takes a file path and decodes it into a slice of
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same logic, just inside a for loop iterating over matches found by the globbing. If it is a single file without glob chars, it will just iterate once on that file.

@@ -522,3 +522,51 @@ func TestDecodeAnyResourceFile(t *testing.T) {
require.Contains(t, platforms, v, "Expected platform label to be one of %v, got %s", platforms, v)
}
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests failed before implementing the fix.

@jsirianni jsirianni marked this pull request as ready for review May 23, 2024 12:08
@jsirianni jsirianni requested a review from tbm48813 May 23, 2024 14:05
@jsirianni jsirianni requested review from dsvanlani and removed request for tbm48813 June 3, 2024 13:30
@jsirianni jsirianni merged commit d1f7103 into main Jun 4, 2024
6 checks passed
@jsirianni jsirianni deleted the fix/input-glob branch June 4, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants